Improve Docker Compose progress output for better user experience#97
Improve Docker Compose progress output for better user experience#97snickerjp wants to merge 8 commits intogetredash:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Redash setup script by implementing a dual-mode progress display system that provides cleaner terminal output during Docker image downloads by default, while maintaining full debugging capabilities via a new --debug flag.
- Adds a
--debugflag (-g) to toggle between clean progress dots and full Docker output - Implements background job execution for
docker compose pullwith dot-based progress indication - Introduces an error handling function that suggests using
--debugfor troubleshooting
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="setup.sh">
<violation number="1" location="setup.sh:389">
P2: Inconsistent error handling: `docker compose up -d` discards stderr, so if it fails users won't see any error details. The previous operations (`docker compose pull`, `docker compose run`) capture and display errors, but this one silently discards them. Consider capturing stderr for consistency with the other docker operations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="setup.sh">
<violation number="1" location="setup.sh:27">
P2: The `local` keyword is not POSIX-compliant. Since this script uses `#!/usr/bin/env sh`, it should avoid bashisms for maximum portability. While `local` works in dash (Debian/Ubuntu) and bash, it may fail on strict POSIX shells. Consider removing `local` and using unique variable names, or change the shebang to `#!/usr/bin/env bash` if bash features are acceptable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Replace cluttered Docker Compose progress output with clean dot-based progress indication by default. Add --debug (-g) option to show detailed Docker progress when needed. Default behavior: - Shows user-friendly messages with dot progress - Clean terminal output Debug mode (--debug): - Shows full Docker Compose progress output - Useful for troubleshooting installation issues
When installation fails in default (clean) mode, show a helpful error message suggesting users run with --debug flag for detailed output. Features: - Clear error indication with emoji - Actionable troubleshooting advice - Maintains clean UX while providing debugging path - Applied to critical Docker Compose operations This helps users quickly identify and resolve installation issues.
Address all 6 issues identified in Copilot review: 1. Add signal traps to prevent process leaks on interruption 2. Fix race condition in exit status handling with explicit STATUS check 3. Apply consistent progress suppression to database creation step 4. Suppress docker compose up output in non-debug mode for consistency 5. Use consistent comparison operators (x = xyes pattern) 6. Preserve error messages using temporary files while suppressing progress Key improvements: - Robust background process management with proper cleanup - Complete progress output suppression in clean mode - Error details automatically shown on failure (--debug less critical) - Maintains clean UX while providing excellent troubleshooting info
Address cubic-dev-ai feedback: docker compose up -d now captures and displays error details consistently with other docker operations. Previously: - docker compose pull/run: captured errors to temp file, displayed details - docker compose up -d: discarded all error output (>/dev/null 2>&1) Now all docker operations consistently capture and display error details while maintaining clean progress output in non-debug mode.
Address Copilot feedback on code duplication: - Extract repeated progress display pattern into run_with_progress() function - Eliminates 40+ lines of duplicated code between pull and create_db operations - Fixes ERROR_LOG variable reuse issue with proper scoping - Improves maintainability and reduces bug risk - Each operation now uses independent ERROR_LOG variables This refactoring maintains identical functionality while significantly improving code quality and maintainability.
Address remaining Copilot feedback: - Reorder help text to match usage line order for better readability - Capture script name at startup to handle symlinks and different invocations - Use SCRIPT_NAME variable in error messages and help text for consistency These improvements enhance user experience and ensure correct script references regardless of how the script is invoked.
Address cubic-dev-ai feedback on POSIX compliance: - Remove 'local' keyword which is a bashism, not POSIX-compliant - Use unique variable names (RWP_MESSAGE, RWP_COMMAND) instead - Maintains compatibility with strict POSIX shells - Script continues to use #!/usr/bin/env sh as intended This ensures the script works on all POSIX-compliant shells, not just bash and dash.
cb80fcb to
0443dac
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…l pull High priority fixes: - Remove unconditional 'docker_compose pull' for offline environment support Docker Compose automatically pulls missing images during 'run' and 'up' Medium priority fixes: - Capture both stdout and stderr to ERROR_LOG for complete error diagnostics Changed from '>/dev/null 2>"$ERROR_LOG"' to '>"$ERROR_LOG" 2>&1' These changes improve offline installation support and error reporting.
Improve Docker Compose progress output for better user experience
Fixes #96
This PR enhances the setup script's user experience by providing cleaner terminal output during Docker image downloads while maintaining full debugging capabilities when needed.
Problem
The current setup script produces cluttered terminal output during Docker image downloads, making it difficult to follow installation progress:
Example of Current Cluttered Output
Solution
Implemented a dual-mode progress display system:
Default Mode (Clean UX)
Debug Mode (
--debugflag)Shows full Docker Compose progress output for troubleshooting.
Key Features
--debugflagError Handling Enhancement
When installation fails in clean mode, users get actionable guidance:
Testing
Benefits
This enhancement significantly improves the user experience while maintaining full debugging capabilities for advanced users and troubleshooting scenarios.